Skip to content
This repository has been archived by the owner on Nov 17, 2023. It is now read-only.

Add magic method abs to NDArray and Symbol. #15680

Merged
merged 7 commits into from
Aug 2, 2019
Merged

Add magic method abs to NDArray and Symbol. #15680

merged 7 commits into from
Aug 2, 2019

Conversation

kshitij12345
Copy link
Contributor

Description

This magic method was missing.
Allow the natural abs(x).

Checklist

Essentials

Please feel free to remove inapplicable items for your PR.

  • Changes are complete (i.e. I finished coding on this PR)
  • All changes have test coverage:
  • Unit tests are added for small changes to verify correctness (e.g. adding a new operator)
  • Nightly tests are added for complicated/long-running ones (e.g. changing distributed kvstore)
  • Build tests will be added for build configuration changes (e.g. adding a new build option with NCCL)
  • Code is well-documented:
  • For user-facing API changes, API doc string has been updated.
  • For new C++ functions in header files, their functionalities and arguments are documented.
  • For new examples, README.md is added to explain the what the example does, the source of the dataset, expected performance on test set and reference to the original paper if applicable
  • Check the API doc at http://mxnet-ci-doc.s3-accelerate.dualstack.amazonaws.com/PR-$PR_ID/$BUILD_ID/index.html
  • To the my best knowledge, examples are either not affected by this change, or have been fixed to be compatible with this change

Changes

  • Add abs magic method.
  • Test for the same.

@kshitij12345 kshitij12345 requested a review from szha as a code owner July 28, 2019 09:39
wkcn
wkcn approved these changes Jul 29, 2019
View reviewed changes
Copy link
Member

@wkcn wkcn left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thank you : )

Could you please add __abs__ for class Symbol in python/mxnet/symbol/symbol.py too? Thx!

@kshitij12345 kshitij12345 force-pushed the develop/ndarray/abs-magic-method branch from 1316651 to 6e64ee9 Compare July 29, 2019 15:41
@kshitij12345
Copy link
Contributor Author

@wkcn Have updated the code. Please review.

@wkcn
Copy link
Member

wkcn commented Jul 29, 2019

@kshitij12345 Great! LGTM. Thanks for your contribution!

@kshitij12345
Copy link
Contributor Author

@wkcn The update for 1 CI check shows that it is pending, however its details show that it has completed successfully. Should I retrigger the CI or is that okay?

@wkcn
Copy link
Member

wkcn commented Jul 31, 2019

@kshitij12345 I have retriggered it : )

@piyushghai
Copy link
Contributor

@mxnet-label-bot Add [pr-awaiting-merge, NDarray]

@marcoabreu marcoabreu added NDArray pr-awaiting-merge Review and CI is complete. Ready to Merge labels Jul 31, 2019
apeforest
apeforest approved these changes Jul 31, 2019
View reviewed changes
Copy link
Contributor

@apeforest apeforest left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for your contribution

@kshitij12345 kshitij12345 changed the title Add magic method abs to NDArray. Add magic method abs to NDArray and Symbol. Aug 1, 2019
@wkcn wkcn merged commit cf28b46 into apache:master Aug 2, 2019
@kshitij12345 kshitij12345 deleted the develop/ndarray/abs-magic-method branch August 2, 2019 04:58
anirudhacharya pushed a commit to anirudhacharya/mxnet that referenced this pull request Aug 20, 2019
@kshitij12345
Add magic method abs to NDArray and Symbol. (apache#15680)
f5b0556 
* add magic method abs to ndarray

* add relevant tests

* add magic method abs to symbol

* add relevant tests

* retrigger CI

* retrigger CI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@piyushghai piyushghai piyushghai approved these changes

@apeforest apeforest apeforest approved these changes

@wkcn wkcn wkcn approved these changes

@szha szha Awaiting requested review from szha szha is a code owner

Assignees
No one assigned
Labels
NDArray pr-awaiting-merge Review and CI is complete. Ready to Merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@kshitij12345 @wkcn @piyushghai @apeforest @marcoabreu